Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

161 search by notation #269

Merged
merged 37 commits into from
Sep 7, 2023
Merged

161 search by notation #269

merged 37 commits into from
Sep 7, 2023

Conversation

sroertgen
Copy link
Contributor

The labels from

  • skos:altLabel
  • skos:notation
  • skos:hiddenLabel
  • skos:definition

are now indexed. By selection of the respective label, only the relevant index gets loaded. This means we don't have one blown up index, but multiple for each label, which makes us handle even quite large vocabs performant.
Also flexsearch has quite improved, so the index is not so big like before.

Re: Accessibility you are now able to check and uncheck boxes using Alt + <first letter of label e.g. Alt + a to toggle search for altLabels. Ctrl + k will you bring back to the search box.

Would like to know if the remaining labels i asked for in #128 should also be added. Since they will not blow up the index by default, I see no problem in doing so.

Will close #128 #161

@sroertgen
WIP: PoC successfull
7749eae 
Need to change to Document indexing instead of "normal" Index.
Then add all the respective labels there.
PoC was kind of successfull, now needs to be polished.
@sroertgen
WIP add checkboxes and load index based on selection
a350920
@sroertgen
style label filter component in two column layout
353f986
@sroertgen
add searchable attributes to config
40f8175
@sroertgen
update eslint rules to v6
3db175c
@sroertgen
add searchable attributes to config
59940ef
@sroertgen
add searchableAttributes to config
ef86e8b
@sroertgen
add support for more searchableAttributes
a748c36 
now attributes that are arrays, languageMapArrays or languageMaps are supported
@sroertgen
ensure search function is actually available
1daba36
@sroertgen
add keypress helpers, refactoring
2bc9322
@sroertgen
try increasing timeout, because github vm is slow
130cff9
@sroertgen
explicitly set firefox as browser
bfe1e64 
the action keeps failing, but works locally totally fine. Maybe adding an explicit browser helps
@sroertgen
try running cypress with chrome
fc9318b
@sroertgen
add wait statement
a3ae364
@sroertgen
lets try headed mode 🤷
1d6b054
@sroertgen sroertgen linked an issue Aug 31, 2023 that may be closed by this pull request
Add strings from skos:altLabel & skos:example to FlexSearch index #128
Closed
@sroertgen sroertgen requested a review from acka47 August 31, 2023 11:54
@sroertgen sroertgen self-assigned this Aug 31, 2023
@sroertgen sroertgen linked an issue Aug 31, 2023 that may be closed by this pull request
Search by notation #161
Closed
@sroertgen
Copy link
Contributor Author

The failing tests, I might handle in a separate issue. Everything goes through locally and I suspect some GitHub Action related issue, since sometimes one cypress test fails and sometimes two.

@sroertgen sroertgen removed the request for review from acka47 August 31, 2023 11:57
@sroertgen
Copy link
Contributor Author

ok, I'm giving up with the cypress GitHub Action thing. Local tests work. I will disable this action for now and open an issue for it.

@sroertgen sroertgen requested a review from acka47 August 31, 2023 13:08
@sroertgen
add additional notation for testing
31ba955
@sroertgen
add id to identify checkbox in tests
a23b0ae
@sroertgen
check if languages are actually defined
533ac48
@sroertgen
replace conceptSchemeId
6bc9c72 
conceptSchemeId was sometimes still having the old value.
This led to unneccessary double rendering.
Removal seems to improve performance and reduce errors with fetching the wrong index
(cause the old conceptSchemeId was still hanging around).
@sroertgen
reenabling e2e test. See if it works now
bc7c662 
I improved testing and also the components. Maybe there was something I overlooked before.
Still don't know why it works locally, but fails in the action.
Let's see if this is still the case.
@sroertgen
Revert "reenabling e2e test. See if it works now"
018a8fd 
This reverts commit bc7c662.
@sroertgen
Copy link
Contributor Author

@acka47 you can test now, e.g. https://test.skohub.io/sroertgen/test-vocabs/heads/main/w3id.org/two-concepts-one-file/c1.de.html

or you can send your own vocab to the test-server

@sroertgen
Copy link
Contributor Author

Telephone protocol:
The searchable attributes should not be directly visible, but should be configurable inside a modal. The modal can be toggled by a button in the search box.

@sroertgen
Copy link
Contributor Author

sroertgen commented Sep 4, 2023 •
edited
Loading

@acka47 please check again.
Attributes are now set in a modal.

@acka47
Copy link
Member

acka47 commented Sep 5, 2023

This looks really good. One last UX thing: I find myself adjusting toggle buttons and then trying to click into the search window to start a search. This won't work at the moment and I have to go back to hit the "close" button first. There are two options for improvements:

  1. Do not require to hit the "close" button and also close the window when you click anywhere else.
  2. Remove the "close" button completely.

acka47
acka47 requested changes Sep 5, 2023
View reviewed changes
Copy link
Member

@acka47 acka47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please adjust the UX as commented

@acka47 acka47 removed their assignment Sep 5, 2023
@sroertgen
Copy link
Contributor Author

@acka47 Outside closing is now implemented.
Please check again, e.g. https://test.skohub.io/sroertgen/test-vocabs/heads/main/w3id.org/two-concepts-one-file/c1.de.html

acka47
acka47 approved these changes Sep 7, 2023
View reviewed changes
Copy link
Member

@acka47 acka47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@sroertgen sroertgen merged commit c0a59e7 into main Sep 7, 2023
2 checks passed
@sroertgen sroertgen deleted the 161-search-by-notation branch September 7, 2023 12:49
@sroertgen sroertgen restored the 161-search-by-notation branch September 8, 2023 14:44
@sroertgen sroertgen deleted the 161-search-by-notation branch February 7, 2024 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acka47 acka47 acka47 approved these changes

Assignees

@acka47 acka47

@sroertgen sroertgen

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Search by notation Add strings from skos:altLabel & skos:example to FlexSearch index
2 participants
@sroertgen @acka47